Unused test data array TEXT_PIXEL_OFFSET_TESTS

Low Severity

The TEXT_PIXEL_OFFSET_TESTS constant defines an array of test cases but is never used. Unlike the other test data arrays in the file (COLOR_TESTS, SIZE_TESTS, TEXT_TESTS) which all have corresponding for...of loops that generate tests, TEXT_PIXEL_OFFSET_TESTS has no such test loop. This dead code adds confusion and maintenance burden.

We're stuck on vitest 2 until we upgrade from node 18.

The RFC outlines the goal being to implement a backwards-compatibility layer and deprecation warnings in @deck.gl/test-utils. I'm not sure what users use it for, but gets a significant number of downloads still so we need to not break anything.

I'm thinking we'll need to keep some old test infrastructure around for this module to ensure correctness on both tape and vitest until deck v10

Temporarily, I've removed tape and probe.gl from deck's test utils until I get all tests to pass without the extra complexity.

Trying to run all of the tests in the node environment resulted in a lot of failures (which is not a surprise), so I'm going to instead implement the same test config for node that we have on master, which is basically a smoke-test.

I'll keep test-fast but remove test-node

I don't think we need ocular-test anymore. Vitest seems to offer everything you need to run tests.

Please let me know if ocular-test did something internally that we lose because we're removing it

Hoping to remove puppeteer after all tests pass

Instead of using a string replace have you considered something like https://github.com/facebook/jscodeshift ?

That way you can operate on the syntax tree and it's a little less hairy than trying to use a regex to parse

Since this is one-time-use and already working reliably, I don’t see a strong reason to change it right now - but I’m open to revisiting if this there's something claude can't figure out

Migration script blindly replaces all .restore() and .reset() calls

Medium Severity

Step 14b uses blanket regexes /\.restore\s*\(\s*\)/g and /\.reset\s*\(\s*\)/g to replace ALL .restore() with .mockRestore() and ALL .reset() with .mockClear(). This transforms any object method named restore or reset, not just spy methods. For example, canvas context save()/restore() pairs, WebGL state management, or any domain object with a reset() method would be incorrectly rewritten. Since the script is designed to be re-run idempotently on future changes, this risks silently breaking non-spy code in any future invocation.

Migration script removes t from all function calls

Medium Severity

Step 11 uses /(\w+)\s*\(\s*t\s*,\s*/g to remove t as the first argument from ANY function call (except a small keyword list). This incorrectly transforms legitimate calls where a variable named t (e.g., a time value, a type parameter, a temporary variable) is passed as a real argument. The keyword exclusion list only covers control flow keywords and misses common functions like expect, Math.min, setTimeout, Array.from, etc. Since this script is idempotent and intended for re-use, this risks silently dropping real arguments in future conversions.

@probe.gl/test-utils peer dependency not marked optional

Medium Severity

The new ./vitest subpath export doesn't depend on @probe.gl/test-utils, yet it remains a required (non-optional) peer dependency. Users who only want the vitest entry point are still forced to install @probe.gl/test-utils. The peerDependenciesMeta marks vitest as optional but omits @probe.gl/test-utils, contradicting the RFC which specifies it should be optional.

Probe.gl peer dependency not marked as optional

Medium Severity

The peerDependenciesMeta only marks vitest as optional, but the RFC explicitly states that @probe.gl/test-utils should also be marked optional. Consumers who only use @deck.gl/test-utils/vitest don't need @probe.gl/test-utils at all, yet package managers will warn about it being missing since it's listed as a required peer dependency. Adding "@probe.gl/test-utils": { "optional": true } to peerDependenciesMeta matches the RFC's design and prevents unnecessary installation warnings.

Piped test commands may mask vitest failures

High Severity

The test, test-fast, test-headless, test-render, test-ci, and test-browser scripts all pipe vitest output through tap-spec (e.g., vitest run ... | tap-spec). In shell pipelines without pipefail, only the exit code of the last command (tap-spec) is used. If vitest crashes or fails to start (no TAP output produced), tap-spec may exit with code 0, silently masking the failure. The CI workflow also uses yarn test-render which has this same pipe, meaning render test infrastructure failures could go undetected.

Migration script mishandles Error constructor throw matchers

Medium Severity

convertThrowsAssertion only recognizes regex patterns (starting with /) as error matchers. Tape's t.throws(fn, TypeError, 'msg') accepts an Error constructor as the second argument, but this function treats any non-regex second argument as a message string. This means TypeError becomes the message, the real message is silently dropped, and .toThrow() is emitted without an argument — producing a weakened assertion that passes for any thrown error instead of checking the error type.

Global .restore() replacement affects non-spy objects

Medium Severity

The regex /\.restore\s*\(\s*\)/g and /\.reset\s*\(\s*\)/g match ANY .restore() or .reset() call on any object, not just spy instances. This could incorrectly transform legitimate method calls — for example, a Canvas2D context's .restore() would become .mockRestore(), and any state object's .reset() would become .mockClear(), breaking the converted test code.

splitArgs escaped character handling differs from findMatchingParen

Low Severity

splitArgs detects escaped quotes by checking if the previous character is a backslash (argsStr[i - 1] !== '\\'), which fails for double-escaped backslashes like \\'. In contrast, findMatchingParen correctly handles this by skipping the character after each backslash (if (str[i] === '\\') i++). This inconsistency means splitArgs could fail to close a string when the closing quote is preceded by an escaped backslash.

Interaction tests run twice in full test suite

Low Severity

The headless project includes test/interaction/**/*.spec.ts, and the render project also includes the same test/interaction/**/*.spec.ts glob. The test script runs both --project headless and then npm run test-render (which uses --project render), causing all interaction tests to execute twice during CI and yarn test. This wastes CI time and could cause confusion if one run passes and the other fails due to timing.

@probe.gl/test-utils missing from root devDependencies breaks tape-compat

Medium Severity

@probe.gl/test-utils was removed from root devDependencies but is still required by: test/smoke/tape-compat.ts (which dynamically imports it), modules/test-utils/src/tape.ts (which statically imports makeSpy), and the main entry index.ts (which re-exports from tape.ts). If not installed transitively via @vis.gl/dev-tools or @probe.gl/bench, the test-tape-compat CI step and the @deck.gl/test-utils main entry point will fail at import time.

Migration script reset() replacement is overly broad

Low Severity

The migration script uses unanchored regex .restore() → .mockRestore() and .reset() → .mockClear() which matches any object's .restore() or .reset() method, not just spy objects. If a test file calls .restore() or .reset() on non-spy objects (e.g., WebGL resources, state objects), the replacement would produce incorrect code. These replacements are not scoped to spy variable names.

@probe.gl/test-utils peer dependency not marked optional

Medium Severity

@probe.gl/test-utils remains a required peer dependency, but the new @deck.gl/test-utils/vitest entry point (vitest.ts) never imports it. Only the tape.ts entry point (used via the default @deck.gl/test-utils import) needs @probe.gl/test-utils. Users who exclusively use @deck.gl/test-utils/vitest will receive unnecessary peer dependency warnings. The peerDependenciesMeta only marks vitest as optional, but @probe.gl/test-utils needs the same treatment.

Interaction tests included in both headless and render projects

Low Severity

test/interaction/**/*.spec.ts is listed in the include of both the headless and render vitest projects. The yarn test command runs --project headless then npm run test-render (which runs --project render), causing all interaction tests to execute twice per full suite run. These tests create shared Deck instances and manipulate browser state, so redundant runs waste CI time and could introduce flaky ordering issues.

Peer dependency not marked optional per RFC design

Medium Severity

The peerDependenciesMeta only marks vitest as optional but does not mark @probe.gl/test-utils as optional. The RFC explicitly specifies @probe.gl/test-utils should be "optional": true in peerDependenciesMeta. Users who only use the @deck.gl/test-utils/vitest entry point don't need @probe.gl/test-utils at all, but will receive peer dependency warnings because it's listed as a required peer dependency.

Interaction tests included in two vitest projects

Low Severity

The test/interaction/**/*.spec.ts glob is included in both the headless and render workspace projects. When yarn test runs (which invokes both --project headless and then --project render), all interaction tests execute twice, wasting CI time. The interaction tests are likely intended only for the render project given they require Playwright browser commands.

Migration script replaces all .restore() and .reset() calls

Low Severity

The migration script uses overly broad regex replacements: ALL .restore() calls become .mockRestore() and ALL .reset() calls become .mockClear(). These patterns match any object's methods, not just spy objects. If any test file has non-spy .restore() or .reset() calls (e.g., canvas context restore() or attribute reset()), re-running this idempotent script would incorrectly transform them.

Migration script blindly converts all .restore() and .reset() calls

Medium Severity

Step 14b uses global regexes /.restore\s*\(\s*\)/g and /.reset\s*\(\s*\)/g that match ANY .restore() or .reset() call, not just spy-related ones. This would incorrectly convert calls like attribute.restore(), buffer.reset(), or any non-spy object method to .mockRestore() or .mockClear(). Since this script is designed to be idempotent and re-runnable, future runs could silently break test files that use .restore() or .reset() on non-spy objects (e.g., GPU resources, WebGL attributes).

Migration script mishandles t.throws with error class matchers

Medium Severity

convertThrowsAssertion only recognizes regex patterns (starting with /) as error matchers. When t.throws(fn, ErrorClass, 'msg') is used with a constructor like Error or TypeError, the second argument is incorrectly treated as the test assertion message. This produces expect(fn, ErrorClass).toThrow() instead of the correct expect(fn, 'msg').toThrow(ErrorClass), silently weakening the assertion by losing the error type check.

Migration removes variable t from arbitrary function calls

Low Severity

Step 11's regex (\w+)\s*\(\s*t\s*,\s* removes t as the first argument from any function call not preceded by a keyword. This doesn't distinguish between the tape test parameter t and a legitimate local variable named t. Calls like setTimeout(t, 100) or Math.min(t, x) where t is a real variable would be incorrectly transformed to setTimeout(100) or Math.min(x).

Probe.gl peer dependency not marked as optional

Medium Severity

@probe.gl/test-utils is a required peer dependency but the RFC specifies it as optional. Only vitest is in peerDependenciesMeta with optional: true. Users who only import from @deck.gl/test-utils/vitest (which never touches probe.gl) will get unnecessary peer dependency warnings from npm/yarn about missing @probe.gl/test-utils. Both @probe.gl/test-utils and vitest need to be optional since each entry point only requires one of them.

Vitest peer dependency version range incompatible with installed version

Medium Severity

The vitest peer dependency in @deck.gl/test-utils is declared as "^2.1.0" (allowing >=2.1.0 <3.0.0), but the project actually uses "vitest": "^4.0.18" in the root devDependencies and all companion packages like @vitest/browser-playwright and @vitest/coverage-v8 are also at ^4.0.18. Vitest 4 has significant breaking changes from v2 (removed Vite 5 support, workspace replaced by projects, stabilized browser mode API changes). Users satisfying this peer dependency with vitest 2.x would encounter incompatibilities with the ./vitest entry point code that was developed and tested against vitest 4.